-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #8783 , cargo new fails without a author name or email #8912
Conversation
If user can not be obtained from git or env variables $USER, new command defaults to empty author in generated Cargo.toml
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Thanks for the PR! Would you be able to add some tests for this? Additionally if we can't find the email or the username then I think we should probably set |
Thank you for looking into it. I will update this PR with unit tests and omit authors from manifest if it is empty. |
- if author name and email not found from config or env variables, defaults to an empty author list authors = [] - simplified selection of name + email from available choices in (fn mk)
- test case for missing name and email (author_without_user_or_email) - test case for handling email only (finds_author_email_only)
Return type changed to plain tuples instead of CargoResult as the function does not bail with error if the author name is missing.
Currently the behavior is to set authors = [] instead of omiting it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just one stylistic nit
src/cargo/ops/cargo_new.rs
Outdated
let author_name = match (cfg.name, discovered_name) { | ||
(Some(name), _) | (_, Some(name)) => Some(name), | ||
(None, None) => None, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this and the below match
can be expressed as cfg.name.or(discovered_name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback and guidance.
Hm it looks like tests may be failing? |
@bors: r+ Thanks! |
📌 Commit a006347 has been approved by |
☀️ Test successful - checks-actions |
Update cargo 10 commits in 63d0fe43449adcb316d34d98a982b597faca4178..d274fcf862b89264fa2c6b917b15230705257317 2020-12-02 01:44:30 +0000 to 2020-12-07 23:08:44 +0000 - Clarify cargo manifest edition field docs (rust-lang/cargo#8953) - Run rustdoc doctests relative to the workspace (rust-lang/cargo#8954) - Workaround fs issue in `cargo publish`. (rust-lang/cargo#8950) - Fix panic with -Zbuild-std and no roots. (rust-lang/cargo#8942) - Slightly optimize `cargo vendor` (rust-lang/cargo#8937) - Fixes rust-lang/cargo#8783 , cargo new fails without a author name or email (rust-lang/cargo#8912) - Fix test escaping __CARGO_TEST_ROOT (rust-lang/cargo#8929) - Add period to allowed feature name characters. (rust-lang/cargo#8932) - faq: small fixes (rust-lang/cargo#8931) - Fix semver documentation tests. (rust-lang/cargo#8930)
If user can not be obtained from git or env variables $USER, new command defaults to empty author in generated Cargo.toml
Could not edit old PR(#8910 8783) as the original clone was deleted.